Skip to content

[DirectX] Overlapping binding detection - check register space first #152250

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 7, 2025

Conversation

hekota
Copy link
Member

@hekota hekota commented Aug 6, 2025

The code that checks for overlapping binding did not compare register space when one of the bindings was for an unbounded resource array, leading to false errors. This change fixes it.

@llvmbot llvmbot added backend:DirectX llvm:analysis Includes value tracking, cost tables and constant folding labels Aug 6, 2025
@llvmbot
Copy link
Member

llvmbot commented Aug 6, 2025

@llvm/pr-subscribers-llvm-analysis

@llvm/pr-subscribers-backend-directx

Author: Helena Kotas (hekota)

Changes

The code that checks for overlapping binding did not compare register space when one of the bindings was for an unbounded resource array, leading to false errors. This change fixes it.


Full diff: https://github.com/llvm/llvm-project/pull/152250.diff

2 Files Affected:

  • (modified) llvm/include/llvm/Analysis/DXILResource.h (+3-1)
  • (added) llvm/test/CodeGen/DirectX/Binding/binding-overlap-7.ll (+35)
diff --git a/llvm/include/llvm/Analysis/DXILResource.h b/llvm/include/llvm/Analysis/DXILResource.h
index 93c6bfb057ef5..7b3e1a19753c8 100644
--- a/llvm/include/llvm/Analysis/DXILResource.h
+++ b/llvm/include/llvm/Analysis/DXILResource.h
@@ -360,9 +360,11 @@ class ResourceInfo {
              std::tie(RHS.RecordID, RHS.Space, RHS.LowerBound, RHS.Size);
     }
     bool overlapsWith(const ResourceBinding &RHS) const {
+      if (Space != RHS.Space)
+        return false;
       if (Size == UINT32_MAX)
         return LowerBound < RHS.LowerBound;
-      return Space == RHS.Space && LowerBound + Size - 1 >= RHS.LowerBound;
+      return LowerBound + Size - 1 >= RHS.LowerBound;
     }
   };
 
diff --git a/llvm/test/CodeGen/DirectX/Binding/binding-overlap-7.ll b/llvm/test/CodeGen/DirectX/Binding/binding-overlap-7.ll
new file mode 100644
index 0000000000000..25f81dd26b9db
--- /dev/null
+++ b/llvm/test/CodeGen/DirectX/Binding/binding-overlap-7.ll
@@ -0,0 +1,35 @@
+; Use llc for this test so that we don't abort after the first error.
+; RUN: not llc %s -o /dev/null 2>&1 | FileCheck %s
+
+; Check that there is no overlap with unbounded array in different space
+
+  ; Buffer<double> A[2] : register(t2, space4);
+  ; Buffer<double> B : register(t20, space5);  // does not overlap
+  ; Buffer<double> C[] : register(t2, space4); // overlaps with A
+
+; CHECK: error: resource A at register 2 overlaps with resource C at register 2 in space 4
+; CHECK-NOT: error: resource C at register 2 overlaps with resource B at register 20 in space 5
+
+target triple = "dxil-pc-shadermodel6.3-library"
+
+@A.str = private unnamed_addr constant [2 x i8] c"A\00", align 1
+@B.str = private unnamed_addr constant [2 x i8] c"B\00", align 1
+@C.str = private unnamed_addr constant [2 x i8] c"C\00", align 1
+
+define void @test_not_overlapping_in_different_spaces() {
+entry:
+
+  ; Buffer<double> A[2] : register(t2, space4);
+  %h0 = call target("dx.TypedBuffer", double, 0, 0, 0)
+            @llvm.dx.resource.handlefrombinding(i32 4, i32 2, i32 2, i32 10, i1 false, ptr @A.str)
+
+  ; Buffer<double> B : register(t20, space5);
+  %h1 = call target("dx.TypedBuffer", i64, 0, 0, 0)
+            @llvm.dx.resource.handlefrombinding(i32 5, i32 20, i32 1, i32 0, i1 false, ptr @B.str)
+
+  ; Buffer<double> C[] : register(t2, space4);
+  %h2 = call target("dx.TypedBuffer", double, 0, 0, 0)
+            @llvm.dx.resource.handlefrombinding(i32 4, i32 2, i32 -1, i32 10, i1 false, ptr @C.str)
+
+  ret void
+}

@hekota hekota requested a review from V-FEXrt August 6, 2025 18:32
@hekota hekota merged commit f7c6c7c into llvm:main Aug 7, 2025
10 checks passed
hekota added a commit that referenced this pull request Aug 11, 2025
…es (#152253)

The resource binding analysis was incorrectly reducing the size of the
`Bindings` vector by one element after sorting and de-duplication. This
led to an inaccurate setting of the `HasOverlappingBinding` flag in the
`DXILResourceBindingInfo` analysis, as the truncated vector no longer
reflected the true binding state.

This update corrects the shrink logic and introduces an `assert` in the
`DXILPostOptimizationValidation` pass. The assertion will trigger if
`HasOverlappingBinding` is set but no corresponding error is detected,
helping catch future inconsistencies.

The bug surfaced when the `srv_metadata.hlsl` and `uav_metadata.hlsl`
tests were updated to include unbounded resource arrays as part of
#145422. These updated test
files are included in this PR, as they would cause the new assertion to
fire if the original issue remained unresolved.

Depends on #152250
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:DirectX llvm:analysis Includes value tracking, cost tables and constant folding
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants